[AMORO-4280] Add support for Lance format on REST metastore - #4334
Conversation
github-actions
Bot
added
module:ams-server
54393c7 to
2502ea9
Compare
2502ea9 to
db3d546
Compare
zhangwl9
changed the title
zhoujinsong
left a comment
zhoujinsong
left a comment
There was a problem hiding this comment.
LGTM.
Thanks a lot for the work!
| && tableFormats.size() > 1), | ||
| "REST catalog serves a single protocol per uri," | ||
| + " Lance cannot be combined with other table formats: %s", | ||
| tableFormats); |
There was a problem hiding this comment.
This check only runs when building a catalog. Updating an already loaded REST catalog bypasses CatalogBuilder, so an API request can persist LANCE,ICEBERG and leave the catalog broken after refresh or restart. Please enforce this invariant before updates are persisted as well.
| if (response == null) { | ||
| return Collections.emptyList(); | ||
| } | ||
| return new ArrayList<>(response.getTables()); |
There was a problem hiding this comment.
The REST response is paginated via pageToken, but this returns only the first page, so larger catalogs silently miss tables. Please follow the token until it is empty; listDatabases() needs the same handling.
| }) | ||
| Object.keys(formState.authConfig).forEach((key) => { | ||
| delete formState.authConfig[key] | ||
| }) |
There was a problem hiding this comment.
Selecting Lance deletes the existing storage/auth values. If the user switches back to Iceberg and saves, those credentials are lost. Since handleSave() already replaces the Lance payload, please preserve the form state here.
3 participants
Why are the changes needed?
Close #4280 .
Brief change log
formats（iceberg、mixed_iceberg）on REST metastore
LanceDirectoryV1Catalog
storage/auth config when REST + LANCE is selected
How was this patch tested?
test by gravitino-lance rest service
Add screenshots for manual tests if appropriate
Run test locally before making a pull request
Documentation